feature: add open telemetry integration #3
Conversation
fcarreno-collab
force-pushed
the
feature/olt_middleware
branch
from
1a9f857 to
fff33eb
Compare
caseylocker
left a comment
caseylocker
left a comment
There was a problem hiding this comment.
You're defining randomString in api/tests/open_telemetry_instrumentation_tests.py but not using it:
class OpenTelemetryInstrumentationTest(APITestCase):
def randomString(self, str_len):
...
Wrapt was downgraded in the requirements.txt. Was that intentional?
wrapt==2.0.1 -> wrapt==1.17.3
@caseylocker I removed this code you are right, its not been used, this was to generate a random string for the access_token we use to authenticate the requests on the other endpoints but here that does not happen
@caseylocker it was on propuse because newer versions create dependency conflicts when we try to install the open telemetry instrumentation libraries |
caseylocker
left a comment
caseylocker
left a comment
There was a problem hiding this comment.
LGTM. I've confirmed that the wrapt downgrade was a requirement by the Opentelementry instrumentation packages. They explicitly require wrapt<2.0.0. No other packages require wrapt 2.x
| elif OTEL_EXPORTER_MODE == "console": | ||
| exporter = ConsoleSpanExporter() | ||
| provider.add_span_processor(BatchSpanProcessor(exporter)) | ||
|
|
There was a problem hiding this comment.
@fcarreno-collab we do need to register
atexit.register(provider.shutdown)
otherwise
With defaults, you'd lose data with Gunicorn worker recycling
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@fcarreno-collab please review comments
4 participants
ref: https://app.clickup.com/t/86b7bdn9g